Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix memory alignment for SIMD #1937

Closed
wants to merge 1 commit into from
Closed

Fix memory alignment for SIMD #1937

wants to merge 1 commit into from

Conversation

falkTX
Copy link

@falkTX falkTX commented Mar 29, 2022

Similar to #1936 this fixes issues regarding memory alignment.
Here these issues can happen on any OS, I had the case where changing Port struct would lead to crashes due to how these were packed in a std::vector.
SSE (and NEON) requires the data it accesses to be aligned, otherwise results in a crash (SIGBUS on ARM).
I guess Rack was lucky that Port contents are able to be packed together in a way that keeps SSE working.

Same with #1936 please take these changes and do with them as you please.
I am here in written text giving you full rights to this patch / code changes.

@falkTX
Fix memory alignment for SIMD
8be9f48 
Signed-off-by: falkTX <falktx@falktx.com>
@falkTX
Copy link
Author

falkTX commented Mar 29, 2022 •
edited
Loading

To add a few notes: while we do not see the issues on current Rack as-is, it is possible to trigger it by changing the Port struct (at least on my side..)
The issue is more prevalent on 32bit systems, like windows 32bit, which some people still like to use in order to keep slightly older but fully functioning hardware alive.
There is a whole conversation about it at DISTRHO/Cardinal#80 where we investigate the issue until finding a solution which this PR is the fix for. A couple plugins need similar changes, I will try to fix them and send PRs their way as well.

PS: Sorry commented this on the wrong ticket.. meant for this specific info to be here and not on #1936

@AndrewBelt
Copy link
Member

We are unfortunately unable to review or accept pull requests to VCV Rack due to copyright reasons regarding unlicensed third-party code in VCV Rack Pro. For more information see https://github.com/VCVRack/Rack/blob/v2/.github/CONTRIBUTING.md. We encourage you to instead contact VCV Support for feature requests and bug fixes.

@AndrewBelt AndrewBelt closed this Apr 25, 2022
@dromer
Copy link

dromer commented Apr 25, 2022

@AndrewBelt There are no "copyright reasons" as @falkTX explicitly gave you full rights to use the diff as you please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@falkTX @AndrewBelt @dromer